feat: add statement coverage instrumentation #879

Wodann · 2025-04-23T15:15:33Z

This PR adds an API for instrumenting statement code coverage, based on this research spike.

Should be merged after #866!

changeset-bot · 2025-04-23T15:15:37Z

🦋 Changeset detected

Latest commit: c09ceaf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@nomicfoundation/edr	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot

Pull Request Overview

This PR introduces statement coverage instrumentation to the project by adding a new API and integrating it into the edr_napi and edr_instrument crates. The changes include updating dependency declarations across Cargo.toml files to use workspace versions, adding the edr_instrument crate with coverage functionality, and exposing the instrumentation API via edr_napi with accompanying tests.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/edr_solidity/Cargo.toml	Updated dependency declarations to use workspace versions.
crates/edr_scenarios/Cargo.toml	Aligned serde dependency with workspace settings.
crates/edr_rpc_eth/Cargo.toml	Transitioned serde and thiserror to workspace configuration.
crates/edr_rpc_client/Cargo.toml	Updated dependency declarations with workspace versions.
crates/edr_provider/Cargo.toml	Adjusted dependency versions to use workspace settings.
crates/edr_op/Cargo.toml	Synced dependency declarations with workspace configurations.
crates/edr_napi_core/Cargo.toml	Updated serde, serde_json, and thiserror to workspace usage.
crates/edr_napi/test/instrument.ts	Added tests validating the instrumentation output.
crates/edr_napi/src/lib.rs	Exposed the new instrument module via the public API.
crates/edr_napi/src/instrument.rs	Implemented the wrapper and conversion for instrumentation results.
crates/edr_napi/index.js	Exported the instrumentation API for use in JavaScript.
crates/edr_napi/Cargo.toml	Updated dependency declarations to align with workspace settings.
crates/edr_instrument/src/lib.rs	Added the instrument module for code coverage instrumentation.
crates/edr_instrument/src/coverage.rs	Implemented the instrumentation logic and tests for deterministic hashing.
crates/edr_instrument/Cargo.toml	Configured edr_instrument with workspace dependencies.
crates/edr_generic/Cargo.toml	Updated dependencies to align with the workspace configuration.
crates/edr_evm/Cargo.toml	Transitioned serde and thiserror to workspace versions.
crates/edr_eth/Cargo.toml	Updated multiple dependencies to workspace-based versions.
Cargo.toml	Added new members and top-level dependency overrides for workspace.
.changeset/lemon-pumas-walk.md	Documented the new instrumentation feature in the changeset.

codecov · 2025-04-23T15:43:58Z

Codecov Report

Attention: Patch coverage is 71.42857% with 58 lines in your changes missing coverage. Please review.

Project coverage is 54.45%. Comparing base (8bcab45) to head (c09ceaf).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/edr_napi/src/instrument.rs	0.00%	54 Missing ⚠️
crates/edr_instrument/src/coverage.rs	97.31%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #879      +/-   ##
==========================================
+ Coverage   54.21%   54.45%   +0.23%     
==========================================
  Files         252      252              
  Lines       29704    29910     +206     
  Branches    29704    29910     +206     
==========================================
+ Hits        16103    16286     +183     
- Misses      12637    12694      +57     
+ Partials      964      930      -34

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

agostbiro

Thanks LGTM with some comments, but none of them blockers.

agostbiro · 2025-04-24T10:24:54Z

crates/edr_instrument/src/coverage.rs

+    content_hash: &B256,
+    statement_counter: u64,
+) -> B256 {
+    let mut hasher = Keccak256::new();


I'm curious about the choice of the hash function. Is the purpose just to generate unique ids deterministically? If yes, we could consider using Blake3 instead of Keccak, as it's an order of magnitude faster: https://github.com/BLAKE3-team/BLAKE3?tab=readme-ov-file#blake3

I started a thread to discuss this internally. If we agree to move forward with Blake3 (an idea I'm all for based on your description) I'll make that change in a follow-up PR such that the Hardhat team can start working with the API earlier.

agostbiro · 2025-04-24T10:26:29Z

crates/edr_instrument/src/coverage.rs

+    })
+
+    // serde_json::to_string_pretty(&metadata)
+    //.context("Failed to serialize metadata")?


Good catch. Addressed in 86e6e12

agostbiro · 2025-04-24T10:27:15Z

crates/edr_instrument/src/lib.rs

@@ -0,0 +1,2 @@
+/// Types for instrumenting code for the purpose of code coverage.
+pub mod coverage;


Could potentially live in edr_solidity

I moved this to a standalone crate as the idea is that this can be shipped in its own NPM package in the future. For the time being - as a shortcut - we're including it in one large NPM package with all other parts of EDR.

I considered including it in edr_solidity but it would require slang to be used as a dependency for that, even when we never would require it for compilation - in the future.

Wodann requested review from a team and Copilot April 23, 2025 15:15

Wodann self-assigned this Apr 23, 2025

Wodann temporarily deployed to github-action-benchmark April 23, 2025 15:15 — with GitHub Actions Inactive

Copilot AI reviewed Apr 23, 2025

View reviewed changes

agostbiro approved these changes Apr 24, 2025

View reviewed changes

Wodann force-pushed the feat/code-coverage branch from cf0340f to e2adefb Compare April 25, 2025 23:09

Wodann force-pushed the feat/instrumentation branch from 42ac089 to 95b7bce Compare April 25, 2025 23:14

Wodann had a problem deploying to github-action-benchmark April 25, 2025 23:14 — with GitHub Actions Failure

Wodann had a problem deploying to github-action-benchmark April 25, 2025 23:17 — with GitHub Actions Failure

Wodann force-pushed the feat/instrumentation branch from 86e6e12 to 53ef3e0 Compare April 25, 2025 23:31

Wodann temporarily deployed to github-action-benchmark April 25, 2025 23:31 — with GitHub Actions Inactive

Wodann force-pushed the feat/code-coverage branch from e2adefb to c046eee Compare April 25, 2025 23:31

Base automatically changed from feat/code-coverage to main April 26, 2025 00:49

feat: add coverage instrumentation

c09ceaf

Wodann force-pushed the feat/instrumentation branch from 53ef3e0 to c09ceaf Compare April 26, 2025 00:53

Wodann temporarily deployed to github-action-benchmark April 26, 2025 00:53 — with GitHub Actions Inactive

Wodann added this pull request to the merge queue Apr 26, 2025

Merged via the queue into main with commit 6f0f557 Apr 26, 2025
17 checks passed

Wodann deleted the feat/instrumentation branch April 26, 2025 00:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add statement coverage instrumentation #879

feat: add statement coverage instrumentation #879

Wodann commented Apr 23, 2025

changeset-bot bot commented Apr 23, 2025 •

edited

Loading

Copilot AI left a comment

codecov bot commented Apr 23, 2025 •

edited

Loading

agostbiro left a comment

agostbiro Apr 24, 2025

Wodann Apr 25, 2025

agostbiro Apr 24, 2025

Wodann Apr 25, 2025

agostbiro Apr 24, 2025

Wodann Apr 25, 2025

		@@ -0,0 +1,2 @@
		/// Types for instrumenting code for the purpose of code coverage.
		pub mod coverage;

feat: add statement coverage instrumentation #879

feat: add statement coverage instrumentation #879

Conversation

Wodann commented Apr 23, 2025

changeset-bot bot commented Apr 23, 2025 • edited Loading

🦋 Changeset detected

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

codecov bot commented Apr 23, 2025 • edited Loading

Codecov Report

agostbiro left a comment

Choose a reason for hiding this comment

agostbiro Apr 24, 2025

Choose a reason for hiding this comment

Wodann Apr 25, 2025

Choose a reason for hiding this comment

agostbiro Apr 24, 2025

Choose a reason for hiding this comment

Wodann Apr 25, 2025

Choose a reason for hiding this comment

agostbiro Apr 24, 2025

Choose a reason for hiding this comment

Wodann Apr 25, 2025

Choose a reason for hiding this comment

changeset-bot bot commented Apr 23, 2025 •

edited

Loading

codecov bot commented Apr 23, 2025 •

edited

Loading